Skip to content

Revert "Cargo.toml: lto = "thin"" to = true#131

Merged
keszybz merged 1 commit intosystemd:mainfrom
nabijaczleweli:ltopsko
Nov 16, 2021
Merged

Revert "Cargo.toml: lto = "thin"" to = true#131
keszybz merged 1 commit intosystemd:mainfrom
nabijaczleweli:ltopsko

Conversation

@nabijaczleweli
Copy link
Collaborator

@nabijaczleweli nabijaczleweli commented Nov 16, 2021 •
edited
Loading

Fat LTO works on bullseye amd64 and i386, sid amd64, i386, and x32, and focal amd64 (I haven't managed to strap impish, something about base-files being SNAFUd or whatever)

Sizing is quite significant in apparent (250k), and still quite good in real (40k):

nabijaczleweli@tarta:~/code/systemd-zram-generator$ l thin true 
-rwxr-xr-x 1 nabijaczleweli users 903K Nov 16 20:21 thin
-rwxr-xr-x 1 nabijaczleweli users 759K Nov 16 20:23 true
nabijaczleweli@tarta:~/code/systemd-zram-generator$ du -h thin true 
441K    thin
405K    true
nabijaczleweli@tarta:~/code/systemd-zram-generator$ size thin true 
   text    data     bss     dec     hex filename
 868823   45361     776  914960   df610 thin
 735002   31137     776  766915   bb3c3 true

This reverts PR #93.

@nabijaczleweli
Revert "Cargo.toml: lto = "thin""
906c186 
lto = true works on bullseye amd64 and i386,
sid amd64, i386, and x32, and focal amd64

This reverts commit 17fc632.
@keszybz
Copy link
Member

keszybz commented Nov 16, 2021

Eh, indeed. I didn't test the sizes myself.

@nabijaczleweli
Copy link
Collaborator Author

nabijaczleweli commented Nov 16, 2021 •
edited
Loading

Hm, actually I didn't look at the reasoning for why I disabled fat LTO (to be fair, it was not in the PR or the commit, only in the actual patch, which the commit/PR didn't link to), and dwz does work on binaries produced by this on i386, but is broken on amd64 and x32.

Fails on x32 with

nabijaczleweli@szarotka:/tmp/zram-generator$ dwz -- target/release/zram-generator
dwz: target/release/zram-generator: Couldn't find DIE at [1d98] referenced by DW_AT_abstract_origin from DIE at [1eec]

and amd64 with

(sid-with-nab sz)root@tarta:/srv/zram-generator# dwz -- target/release/zram-generator
dwz: target/release/zram-generator: Couldn't find DIE at [1eaf] referenced by DW_AT_abstract_origin from DIE at [1f6a]

that being said, this is a downstream issue, since we don't run dwz anywhere, downstreams that do can (selectively) apply a patch to fall back to thin lto, but there's no reason to penalise other installs like this.

Oh, and dwz only fails when built with 1.56.0 or earlier (the ones from the archive, anyway). I have a rustup-originated rustc 1.56.1 and it works just fine on amd64, so? There's a workaround (rust-lang/rust#89041) merged, but unreleased, so this will also cease to be a problem entirely in 1.57.0.

@keszybz
Copy link
Member

keszybz commented Nov 16, 2021

FWIW, with rust-1.56.1-1.fc35.x86_64 and dwz-0.14-2.fc35.x86_64, I don't see the issue. So yeah, I think it's fine to merge this. I was just waiting for the CI to finish.

@keszybz keszybz merged commit 3ba0006 into systemd:main Nov 16, 2021
@keszybz keszybz removed their request for review November 16, 2021 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nabijaczleweli @keszybz